Add new "Mille" package to externals, update Millepede-II and GBL #10310
Add new "Mille" package to externals, update Millepede-II and GBL #10310cmsbuild merged 8 commits intocms-sw:IB/CMSSW_16_1_X/masterfrom
Conversation
|
cms-bot internal usage |
|
A new Pull Request was created by @goblirsc for branch IB/CMSSW_16_1_X/master. @akritkbehera, @cmsbuild, @iarspider, @raoatifshad, @smuzaffar can you please review it and eventually sign? Thanks. |
|
please test with cms-sw/cmssw#49963 |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b8b998/50978/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Comparison SummarySummary:
Max Memory Comparisons exceeding threshold@cms-sw/core-l2 , I found 2 workflow step(s) with memory usage exceeding the error threshold: Expand to see workflows ...
|
|
|
||
| %prep | ||
| %setup -q -n %{n}-%{realversion} | ||
| grep -q 'CMAKE_CXX_STANDARD *11' cpp/CMakeLists.txt |
There was a problem hiding this comment.
@goblirsc , please do not delete this line. It is there to make sure that the cpp standard substitution we are doing in the next line works. I see that new gbl has default standard 17 ( https://gitlab.desy.de/millepede/general-broken-lines/-/blob/V04-00-00/cpp/CMakeLists.txt?ref_type=tags#L47) , so please add this line back and update the expected standard i.e.
grep -q 'CMAKE_CXX_STANDARD *17' cpp/CMakeLists.txt
There was a problem hiding this comment.
added lines back and in both cased specified 17 as the standard to replace
| -DCMAKE_VERBOSE_MAKEFILE=ON \ | ||
| -DEIGEN3_INCLUDE_DIR=${EIGEN_ROOT}/include/eigen3 \ | ||
| -DSUPPORT_ROOT=False \ | ||
| -DCMAKE_CXX_FLAGS="$CMS_EIGEN_CXX_FLAGS %{selected_microarch}" |
mille.spec
Outdated
| rm -rf build | ||
| mkdir build | ||
| cd build | ||
| cmake \ | ||
| -DCMAKE_INSTALL_PREFIX=%{i} \ | ||
| ../ | ||
| make | ||
|
|
||
| %install | ||
| cd build | ||
| make install PREFIX=%{i} |
There was a problem hiding this comment.
| rm -rf build | |
| mkdir build | |
| cd build | |
| cmake \ | |
| -DCMAKE_INSTALL_PREFIX=%{i} \ | |
| ../ | |
| make | |
| %install | |
| cd build | |
| make install PREFIX=%{i} | |
| rm -rf ../build | |
| mkdir ../build | |
| cd ../build | |
| cmake \ | |
| DCMAKE_BUILD_TYPE=Release \ | |
| -DCMAKE_INSTALL_PREFIX=%{i} \ | |
| -DCMAKE_PREFIX_PATH="%{cmake_prefix_path}" | |
| ../%{n}-%{realversion} | |
| make %{makeprocesses} VERBOSE=1 | |
| %install | |
| cd ../build | |
| make install PREFIX=%{i} |
to avoid building in the source tree and also build in parallel & verbose modemake %{makeprocesses} VERBOSE=1
millepede.spec
Outdated
| -DCMAKE_INSTALL_PREFIX=%{i} \ | ||
| -DLAPACK_OPENBLAS=off \ | ||
| ../ | ||
| make |
There was a problem hiding this comment.
same here, build outside the source tree i.e. try using ../build and also update cmake configuration to explicitly set CMAKE_PREFIX_PATH and CMAKE_BUILD_TYPE . Update make to run in parallel
There was a problem hiding this comment.
thanks - should be fixed in the latest commit
|
@goblirsc , couple of files also need relocation ( see https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b8b998/50978/external_checks/relocate/ ). so please update the specs to relocate these too |
| @@ -0,0 +1,25 @@ | |||
| ### RPM external mille V01-00-00 | |||
| ## INCLUDE cpp-standard | |||
There was a problem hiding this comment.
@goblirsc , if we do not want to override the c++ standard for mille then there is no need to have this line. The above line only sets the default c++std for cms software stack and packages which are sensitive to c++std can make use of it. So if you need to build mille for default cms c++std then keep this line but then also add -DCMAKE_CXX_STANDARD=%{cms_cxx_standard} to cmake
There was a problem hiding this comment.
thanks, now passing the cms_cxx_standard to CMake
|
Pull request #10310 was updated. |
|
please test with cms-sw/cmssw#49963 |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b8b998/51090/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Comparison SummarySummary:
|
|
please test with cms-sw/cmssw#49963 for el9_amd64_gcc15 just to make sure it works for newer compilers and c++23 standard |
|
REMINDER @mandrenguyen, @sextonkennedy, @ftenchini: This PR was tested with cms-sw/cmssw#49963, please check if they should be merged together |
at a quick glance, there seem a huge amount of static analysis errors from unrelated packages - is the existing codebase expected to pass? |
|
+externals gcc15 static analyzer errors are not related to this change. |
|
This pull request is fully signed and it will be integrated in one of the next IB/CMSSW_16_1_X/master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @mandrenguyen, @sextonkennedy, @ftenchini (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
cf4fe20
into
cms-sw:IB/CMSSW_16_1_X/master
This PR updates the tracker alignment packages Millepede-II and GBL to use a new, common implementation of the "Mille" library responsible for writing and reading the alignment fit inputs to/from disk. The common Mille package is added as a new external package, and dependencies on it introduced to the Millepede-II and GBL spec files.
See slides for details on the new package.
Changes require updates to CMSSW, as some API calls change. These are in PR #49963.
Together, the changes pass the unit and runTheMatrix tests (few exceptions related to DAS / missing inputs, unrelated to PR).
In addition, changes were validated by running a full tracker alignment with the modified externals & CMSSW on lxplus8.